Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable lifi cross-account trades #5835

Merged
merged 2 commits into from
Dec 12, 2023
Merged

Conversation

0xApotheosis
Copy link
Member

@0xApotheosis 0xApotheosis commented Dec 12, 2023

Description

Add cross-account trading to LifI.

Cross-account LiFi transactions created from this branch:

Pull Request Type

  • 🐛 Bug fix (Non-breaking Change: Fixes an issue)
  • 🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
  • 💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

Relates to #4966

Risk

Medium. It's a small change, but as always, touching account selection is risky as mistakes can mean lost funds.

Testing

Confirm that Li.Fi quotes now show up when trading between accounts, and that they and sent and received from the correct accounts when executed.

Engineering

Sanity check logic.

Operations

☝️

Screenshots (if applicable)

Screenshot 2023-12-12 at 11 30 13 am

@0xApotheosis 0xApotheosis requested a review from a team as a code owner December 12, 2023 00:34
Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conceptual stamp, haven't managed to consistently get Li.Fi quotes (or any quote for that matter) because of our current timeout issues for the last , but let's retest this and/or get this ops tested if things are looking better in this regard tonight 🙏🏽

Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On @woodenfurniture's recommendation, managed to test (after many many tries, fees estimation are still turbo long and borked) with Avalanche as Gnosis fees estimation is all kinds of slow and borked atm, got lucky with it when testing 1inch cross-account but seems borked now.

Account 0 -> 1

image image image image

Note, while not strictly related to this PR, found some interesting discrepancy in the underlying tool being used by the Li.Fi routing vs. the one announced (cc @woodenfurniture):

image image

Account 1 -> 0

image image image image

@gomesalexandre gomesalexandre merged commit 10fca9c into develop Dec 12, 2023
3 checks passed
@gomesalexandre gomesalexandre deleted the cross-account-lifi branch December 12, 2023 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants